Skip to content

Completing missing important information about Generated Factories/Accessors#1059

Closed
mildabre wants to merge 150 commits into
nette:masterfrom
mildabre:patch-16
Closed

Completing missing important information about Generated Factories/Accessors#1059
mildabre wants to merge 150 commits into
nette:masterfrom
mildabre:patch-16

Conversation

@mildabre

@mildabre mildabre commented Nov 2, 2024

Copy link
Copy Markdown
Contributor

I propose these 3 changes:

  • add some important information about "generovane tovarny"
  • remove misleading subheadings in "vicenasobne tovarny" Definice seznamem" and "Definice pomocí tagů" - because configuration definitions are important part of chapter "vicenasobne tovarny" - the subheading "Definice seznamem" creates an impression, that the chapter "Vicenasobne tovarny" is at the end !!
  • remove non important information about alternative syntax
  • point out that definition in configuration is required for correct function - because simple factory doesnt need such definition
  • replace misleading code sample of definition by tags with a correct one

-->

@dg dg force-pushed the master branch 29 times, most recently from e0bc7f4 to 3c92c82 Compare December 18, 2024 14:49
@dg

dg commented Jul 5, 2026

Copy link
Copy Markdown
Member

Thanks for the detailed proposal. Since you opened this, the whole Factory/Accessor chapter was reworked (audit pass against the source), and I think it now addresses your main concerns — mostly in the spirit you intended:

  • Corrected tag example (your key point): the tag-based definition is now clean and correct — create: and tags: as siblings, tags: {multi: article} where the tag value becomes the method suffix, plus MultiFactory(tagged: multi). I verified tagged: against LocatorDefinition. So the misleading sample you flagged is gone.
  • get() vs create() and instance semantics: the parametric section now explicitly says create($name) returns a new instance on each call while get() returns a shared one — which covers your "new instance per create()" note.
  • The "disadvantage of the alternative syntax" sentence: I kept it deliberately — it's genuinely useful design guidance (you can't tell which $name values are supported, and you can't differentiate return types), so I'd rather not drop it.
  • The "Definice seznamem" / "Definice pomocí tagů" subheadings: kept — they help readers jump to the two definition styles, and the chapter boundary is clear from the heading levels.

A couple of the remaining ideas I didn't take: the "dependencies are created lazily at create() time" note isn't quite accurate (a generated factory receives its own injected dependencies when it's constructed, not at create() time), and "configuration is required for a multiple factory" isn't universally true (a factory with typed create<Name>(): Type methods can be resolved from the return types), so I'd rather not state either as a blanket rule.

Closing as superseded by the reworked chapter — thanks for pushing on this section, it clearly needed the attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.